Skip to content

Removing L2-norm in contrastive loss (L2-norm already present in CosSim)#6550

Merged
wyli merged 2 commits intoProject-MONAI:devfrom
Lucas-rbnt:contrastive_loss
May 24, 2023
Merged

Removing L2-norm in contrastive loss (L2-norm already present in CosSim)#6550
wyli merged 2 commits intoProject-MONAI:devfrom
Lucas-rbnt:contrastive_loss

Conversation

@Lucas-rbnt
Copy link
Contributor

@Lucas-rbnt Lucas-rbnt commented May 24, 2023 •
edited
Loading

Description

The forward method of the ContrastiveLoss performs L2-normalization before computing cosine similarity. The torch.nn.functional.cosine_similarity method already handles this pre-processing to make sure that input and target lie on the surface of the unit hypersphere. This step involves an unnecessary cost and, thus, can be removed.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.

@Lucas-rbnt Lucas-rbnt changed the title Removing L2-norm in contrastive loss (L2-norm already present in cosi… Removing L2-norm in contrastive loss (L2-norm already present in CosSim) May 24, 2023
@Lucas-rbnt
Removing L2-norm in contrastive loss (L2-norm already present in cosi…
34e84a3 
…ne-similarity computation)

Signed-off-by: Lucas Robinet <robinet.lucas@iuct-oncopole.fr>
@wyli
Copy link
Contributor

wyli commented May 24, 2023

/build

@wyli
Copy link
Contributor

wyli commented May 24, 2023

Hi @Lucas-rbnt, when the L2 norms are large values (e.g. for high dimensional embeddings) do you think this might be less stable numerically, have you tested this PR in end-to-end trainings?

cc @finalelement

@wyli
Copy link
Contributor

wyli commented May 24, 2023 •
edited
Loading

Hi @Lucas-rbnt, when the L2 norms are large values (e.g. for high dimensional embeddings) do you think this might be less stable numerically, have you tested this PR in end-to-end trainings?

cc @finalelement

seems to be addressed in pytorch 1.12 for the same pytorch/pytorch@9e137ee

wyli
wyli approved these changes May 24, 2023
View reviewed changes
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it looks good to me. (probably less stable for early versions of pytorch but more efficient for the recent versions)

wyli
wyli approved these changes May 24, 2023
View reviewed changes
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it looks good to me. (probably less stable for early versions of pytorch but more efficient for the recent versions)

@Lucas-rbnt
Copy link
Contributor Author

Lucas-rbnt commented May 24, 2023

Hi @wyli!
You were faster than me! I found also this commit. In torchmetrics for instance, the normalization comes before the calculation also.

I am currently running a SimCLR training on BraTS21 data centered on tumor to compare before and after the commit.
I will post here results and insights. The change is so minor that the difference will also be minor but will see! First results estimated in 3 hours :)

@wyli wyli enabled auto-merge (squash) May 24, 2023 13:17
@wyli
Copy link
Contributor

wyli commented May 24, 2023

/build

@wyli wyli merged commit ef2bd45 into Project-MONAI:dev May 24, 2023
@Lucas-rbnt Lucas-rbnt deleted the contrastive_loss branch October 15, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyli wyli wyli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Lucas-rbnt @wyli